Skip to content

fix: batch ContactCard/get to respect maxObjectsInGet#46

Open
capitanroy wants to merge 2 commits intoroot-fr:mainfrom
capitanroy:fix/contacts-pagination-maxobjectsinget
Open

fix: batch ContactCard/get to respect maxObjectsInGet#46
capitanroy wants to merge 2 commits intoroot-fr:mainfrom
capitanroy:fix/contacts-pagination-maxobjectsinget

Conversation

@capitanroy
Copy link
Copy Markdown

Summary

Fixes #45 — contacts fail to load silently when the address book contains more entries than the server's maxObjectsInGet limit.

  • Query contact IDs first, then check count against maxObjectsInGet from session capabilities
  • If within the limit, use the existing efficient back-reference pattern (no behavior change)
  • If over the limit, split into batches and concatenate results
  • Adds getMaxObjectsInGet() helper following the existing pattern of getMaxCallsInRequest() / getMaxSizeUpload()

Context

Stalwart's default maxObjectsInGet is 500 (per RFC 8620). When the address book has >500 contacts, ContactCard/get with a back-reference to all queried IDs returns requestTooLarge. The error is caught and the contact list renders as empty with no user feedback.

Test plan

  • Verified with Stalwart (maxObjectsInGet: 500) and 598 contacts
  • Contacts load correctly after fix (batched into 2 requests)
  • <500 contacts still use the single back-reference pattern (no regression)

When the address book contains more contacts than the server's
maxObjectsInGet limit (default 500 in Stalwart), the back-referenced
ContactCard/get call fails with requestTooLarge. The error is caught
silently, resulting in an empty contact list.

Fix: query IDs first, then check against maxObjectsInGet. If within
the limit, use the efficient back-reference pattern. Otherwise, split
into batches and concatenate results.

Also adds getMaxObjectsInGet() helper following the existing pattern
of getMaxCallsInRequest() and getMaxSizeUpload().

Fixes root-fr#45
Copy link
Copy Markdown
Contributor

@ma2t ma2t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @capitanroy — this is a real bug and the overall approach (query IDs first, then batch) is solid. The getMaxObjectsInGet() helper follows existing patterns perfectly, and the PR description is excellent.

A few things to address before merging:


1. Redundant query in the single-batch path

When allIds.length <= maxBatchSize, the code re-issues the same ContactCard/query just to use the back-reference pattern. This means every contact load now costs 2 HTTP roundtrips instead of 1 — even for users with <500 contacts (the common case).

Since allIds is already available from the first query, the single-batch path can just pass the IDs directly:

if (allIds.length <= maxBatchSize) {
  const response = await this.request([
    ["ContactCard/get", { accountId, ids: allIds }, "0"],
  ], this.contactUsing());

  if (response.methodResponses?.[0]?.[0] === "ContactCard/get") {
    return (response.methodResponses[0][1].list || []) as ContactCard[];
  }
  return [];
}

2. Multi-batch: pack into a single JMAP request

Each batch currently issues a separate this.request() — N sequential HTTP roundtrips. RFC 8620 allows multiple method calls per request (up to maxCallsInRequest, default 50). All batches can be packed into one request:

const calls: [string, Record<string, unknown>, string][] = [];
for (let i = 0; i < allIds.length; i += maxBatchSize) {
  const batchIds = allIds.slice(i, i + maxBatchSize);
  calls.push(["ContactCard/get", { accountId, ids: batchIds }, String(calls.length)]);
}

const response = await this.request(calls, this.contactUsing());
const allContacts: ContactCard[] = [];
for (const [method, result] of response.methodResponses || []) {
  if (method === "ContactCard/get") {
    allContacts.push(...(result.list || []));
  }
}
return allContacts;

For 1200 contacts / 500 batch = 3 method calls — well within maxCallsInRequest, single roundtrip.

3. Tests need updating

The existing tests in jmap-contact-client.test.ts mock the old [query, get] two-method-in-one-request pattern. The new flow will break them. Please add/update tests covering:

  • < maxObjectsInGet contacts (single-batch path)
  • > maxObjectsInGet contacts (multi-batch path)
  • 0 contacts
  • Exact boundary (allIds.length === maxBatchSize)

4. Minor: limit: 1000 caps results silently (pre-existing)

Not introduced by this PR, but now more visible: the query has limit: 1000, so users with >1000 contacts will get a silently truncated list — the same class of issue this PR fixes. Worth a // TODO: paginate query for >1000 contacts comment so it doesn't get forgotten.


Happy to answer any questions. The core logic is right — just needs these adjustments and it's good to go.

ma2t added a commit that referenced this pull request Mar 23, 2026
New features:
- Folder management with context menu, inline editing, drag-and-drop (#44)
- Mail multi-selection with batch move/delete and shift-click (#43)

Bug fixes:
- Health endpoint false-positive restarts (#41, thanks @wrenix and @ClemaX)
- Identity deletion failing (#42, thanks @freddij)
- Inline CID images, email list flicker, dark mode clipboard tint

Feature requests from @dlecourtaltimafr (#43, #44).
Contact pagination fix contributed by @capitanroy (#46).

Dependencies: Next.js 16.2.1, Tailwind 4.2.2, Zustand 5.0.12,
flatted CVE fix (GHSA-rf6f-7fwh-wjgh).
- Single-batch path: pass queried IDs directly instead of re-issuing
  the query with a back-reference (removes extra HTTP roundtrip for
  the common <500 contacts case)
- Multi-batch path: pack all ContactCard/get calls into one JMAP
  request with multiple method calls (single roundtrip regardless of
  contact count, per RFC 8620 maxCallsInRequest)
- Add TODO for pre-existing limit:1000 silent truncation
- Update tests for new two-request flow with boundary/batch coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Contacts fail silently when count exceeds maxObjectsInGet

2 participants